Skip to content

fix: add version constraint to utils/nextflow env yaml #4637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: add version constraint to utils/nextflow env yaml #4637

wants to merge 1 commit into from

Conversation

@tedil
Copy link
Contributor

@tedil tedil commented Oct 8, 2025 •
edited by coderabbitai bot
Loading

As a less invasive alternative to snakemake/snakedeploy#107 this PR adds a version constraint to the nextflow version in the environment.yaml so snakedeploy update-conda-envs --pin-envs can actually pick up changes.

Summary by CodeRabbit

  • Chores
    • Pinned Nextflow to version 25.04.8 for consistent, repeatable environments.
    • Refreshed Linux environment pins with updated packages and libraries to improve compatibility and stability.
    • Added metadata to the environment pin file for clearer provenance.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 8, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

Updated Nextflow environment configuration: pinned Nextflow to version 25.04.8 in environment.yaml and regenerated the explicit linux-64 conda lock/pin file with updated package URLs, versions, and metadata.

Changes

Cohort / File(s) Summary
Nextflow Conda pin set (linux-64)
utils/nextflow/environment.linux-64.pin.txt		 Replaced prior explicit pins with a new, expanded set of linux-64 packages and versions; added a created-by metadata line; numerous system/library/tooling updates and removals with updated URLs.
Environment spec version pin
utils/nextflow/environment.yaml		 Changed dependency to pin Nextflow exactly: from - nextflow to - nextflow =25.04.8.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description explains the motivation and change but omits the required QC checklist and confirmation of guideline compliance defined in the repository’s PR template, leaving out the section for ticking off the contributing and testing requirements. Please update the PR description to include the QC section from the template with the checklist items for documentation adherence, test updates, and conda environment minimal channel usage, and mark the relevant boxes to confirm guideline compliance.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly summarizes the primary change by indicating a fix that adds a version constraint to the Nextflow environment YAML under utils/nextflow and follows the conventional commit “fix:” style. It is concise, directly related to the main update in the pull request, and provides sufficient context for a reviewer skimming the history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch utils/nextflow/add-constraints
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 502ef25 and 5294ed7.

📒 Files selected for processing (2)
  • utils/nextflow/environment.linux-64.pin.txt (1 hunks)
  • utils/nextflow/environment.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: testing
  • GitHub Check: Summary
🔇 Additional comments (1)
utils/nextflow/environment.yaml (1)

6-6: Fix match spec spacing for conda.

Line 6 currently spells the dependency as nextflow =25.04.8. Conda’s dependency specs don’t permit spaces around the = operator; leaving the space causes the solver to treat the entry as a literal string and the environment creation fails. Please tighten it to nextflow=25.04.8.

-- nextflow =25.04.8
+- nextflow=25.04.8
⛔ Skipped due to learnings 
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#4159
File: bio/pyfaidx/environment.yaml:6-6
Timestamp: 2025-06-02T07:56:35.854Z
Learning: In the Snakemake-wrapper repository, conda dependency version pinning in environment.yaml files uses spaces around the equals sign (e.g., `- pyfaidx =0.8.1.4`) as the established coding standard, even though conda itself doesn't require the spaces.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@fgvieira fgvieira enabled auto-merge (squash) October 8, 2025 15:23
@tedil
Copy link
Contributor Author

tedil commented Oct 9, 2025 •
edited
Loading

@fgvieira Any clue why the tests fail? Depending on run, we get permission errors on different binaries, e.g.,: ERROR test_wrappers.py::test_nextflow - PermissionError: [Errno 1] Operation not permitted: '/tmp/tmph3a872do/tmp8_ww334l/test/work/1a/290ea65c12de7b2287946c9c37d146/size_factors'

@fgvieira
Copy link
Collaborator

fgvieira commented Oct 9, 2025

Not very familiar with nextflow but re-running the same test, gives a different error:
ERROR test_wrappers.py::test_nextflow - PermissionError: [Errno 1] Operation not permitted: '/tmp/tmpalhsifqj/tmpxs93imm6/test/work/a3/f167abb4500d9e1f3ca22d09bf4427/multiqc_plots'

Maybe something to do with temp space?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fgvieira fgvieira fgvieira approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tedil @fgvieira